Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Try default gcc 9.4.0 to see if it exhibits the same compiler bugs. #8394

Merged
merged 2 commits into from
Sep 8, 2023

Conversation

ioquatix
Copy link
Member

@ioquatix ioquatix commented Sep 7, 2023

If #8393 works, let's try gcc-9 to see if that mitigates the issue too. Perhaps this was a bug introduced in gcc-10.

@ioquatix ioquatix force-pushed the ppc64le-older-compiler branch 2 times, most recently from d439927 to 620f279 Compare September 7, 2023 13:16
@junaruga
Copy link
Member

junaruga commented Sep 7, 2023

Thanks for the work!

@ioquatix
Copy link
Member Author

ioquatix commented Sep 7, 2023

@junaruga this is now passing:

image

I'm going to try merging this.

Hopefully #8393 continues to be okay.

@junaruga
Copy link
Member

junaruga commented Sep 8, 2023

Using gcc-9 makes sense for now, because we use gcc (version 9.4.0) on Ruby CI Ubuntu ppc64le and s390x. So, I agree using the gcc-9.

And the PR's Travis CI is passing.
https://app.travis-ci.com/github/ruby/ruby/builds/265789165

However, I see the used gcc version is actually not gcc-9 but gcc (= version 9.4.0) in the Travis CI log. Because while the &gcc-10 exists in .travis.yml, the &gcc-9 doesn't exist in it.
https://app.travis-ci.com/github/ruby/ruby/jobs/609434805#L139

$ gcc --version
gcc (Ubuntu 9.4.0-1ubuntu1~20.04.1) 9.4.0
Copyright (C) 2019 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

So, I think a possible fix is to add the &gcc-9 referred by *gcc-9 to explicitly showing the used gcc is gcc-9.

$ git diff .travis.yml
diff --git a/.travis.yml b/.travis.yml
index c6054ae672..3cbe921762 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -72,6 +72,24 @@ env:
         openssl
         zlib1g-dev
 
+  - &gcc-9
+    compiler: gcc-9
+    before_install:
+      - tool/travis_retry.sh sudo bash -c "rm -rf '${TRAVIS_ROOT}/var/lib/apt/lists/'* && exec apt-get update -yq"
+      - >-
+        tool/travis_retry.sh sudo -E apt-get $travis_apt_get_options install
+        ccache
+        gcc-9
+        g++-9
+        libffi-dev
+        libncurses-dev
+        libncursesw5-dev
+        libreadline-dev
+        libssl-dev
+        libyaml-dev
+        openssl
+        zlib1g-dev
+
   # --------
 
   - &arm64-linux

Or to use the default gcc (= version 9.4.0). I like this way, because it's simple and is aligning with the current RubyCI Ubuntu ppc64le and s390x.

$ git diff
diff --git a/.travis.yml b/.travis.yml
index c6054ae672..11ae8a9b45 100644
--- a/.travis.yml
+++ b/.travis.yml
@@ -82,12 +82,12 @@ env:
   - &ppc64le-linux
     name: ppc64le-linux
     arch: ppc64le
-    <<: *gcc-10
+    compiler: gcc
 
   - &s390x-linux
     name: s390x-linux
     arch: s390x
-    <<: *gcc-10
+    compiler: gcc
 
   - &arm32-linux
     name: arm32-linux

What do you think? Could you change the code?

@junaruga junaruga self-requested a review September 8, 2023 05:54
@junaruga
Copy link
Member

junaruga commented Sep 8, 2023

When we fix on the 2nd way, I think the PR title can be like this.

"Try the system gcc 9.4.0 to see if it exhibits the same compiler bugs."

or

"Try the default gcc 9.4.0 to see if it exhibits the same compiler bugs."

@junaruga
Copy link
Member

junaruga commented Sep 8, 2023

Can you change the commit message in the 2 commits to change the gcc-9 to gcc, including the keyword "Travis" and "ppc64le" and "s390x" and explaining the context and reasons? I think squashing the 2 commits to 1 commit is better, because it comes from the same reason, right?

The message can be like this. You can check how the commit messages are written in the past in these kind of cases by the $ git log .travis.yml.

.travis.yml: Try gcc 9.4.0 instead of gcc-10 in the ppc64le and s390x cases.

Use gcc 9.4.0 instead of gcc-10 to avoid failures in the ppc64le and s390x cases and align with RubyCI Ubuntu ppc64le and s390x.

Copy link
Member

@junaruga junaruga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I commented. Thanks!

@ioquatix
Copy link
Member Author

ioquatix commented Sep 8, 2023

I think you can commit to my branch if you want. Let me give you access.

@junaruga
Copy link
Member

junaruga commented Sep 8, 2023

I think you can commit to my branch if you want. Let me give you access.

I am a bit sick today. So, if you want to merge this PR today, I hope you change the PR by yourself. If you are fine to merge this PR next week, I try to update your branch. I am not sure if I can update your branch today.

@junaruga
Copy link
Member

junaruga commented Sep 8, 2023

@ioquatix I changed my mind. I am working to update your branch now.

@junaruga junaruga changed the title Try gcc-9 to see if it exhibits the same compiler bugs. Try default gcc 9.4.0 to see if it exhibits the same compiler bugs. Sep 8, 2023
@junaruga
Copy link
Member

junaruga commented Sep 8, 2023

I rebased the PR. Let's see the results of the Travis CI

I confirmed the rebased commit uses the default gcc 9.4.0 as I expected in my forked repository's Travis in advance.
https://app.travis-ci.com/github/junaruga/ruby/builds/265795672

@junaruga junaruga changed the title Try default gcc 9.4.0 to see if it exhibits the same compiler bugs. Try default gcc 9.4.0 to see if it exhibits the same compiler bugs. Sep 8, 2023
…390x.

Use gcc 9.4.0 instead of gcc-10 to avoid the current failures by a possible GCC
10 compiler bug in the Travis ppc64le and s390x cases. And it also aligns with
RubyCI Ubuntu ppc64le and s390x where the default gcc is used.

Co-authored-by: Jun Aruga <jaruga@ruby-lang.org>
@junaruga
Copy link
Member

junaruga commented Sep 8, 2023

All right. I executed Travis 2 times for this PR on ruby/ruby Travis. And I see ppc64le and 390x cases pass in the both 1st and 2nd time.

https://app.travis-ci.com/github/ruby/ruby/builds/265796147
https://app.travis-ci.com/github/ruby/ruby/builds/265795840

@ioquatix ioquatix merged commit be21a05 into ruby:master Sep 8, 2023
92 checks passed
@ioquatix ioquatix deleted the ppc64le-older-compiler branch September 8, 2023 08:44
mame added a commit to ruby/chkbuild that referenced this pull request Sep 8, 2023
mame added a commit to ruby/chkbuild that referenced this pull request Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants